-
Notifications
You must be signed in to change notification settings - Fork 22
Coinbase Ruby guide #304
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Coinbase Ruby guide #304
Conversation
| how to disable API class loading on the Coinbase Ruby side if needing to use both dependencies in the same project since | ||
| two sets of API classes cannot both be present. | ||
|
|
||
| Migration cannot be done on a live, running workflow. Overall, Coinbase Ruby workflow events are incompatible with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would our SDK be able to replay history from Coinbase's SDK? or are we just able to translate WF code from one SDK to another, as in our SDK can run Coinbase workflows and vise versa.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would our SDK be able to replay history from Coinbase's SDK?
No, Coinbase Ruby workflow events are incompatible with Temporal Ruby workflow events at runtime. Our SDK can start and interact with Coinbase workflows and vice-versa, yes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a bit of clarity here
README.md
Outdated
| two sets of API classes cannot both be present. | ||
|
|
||
| Migration cannot be done on a live, running workflow. Overall, Coinbase Ruby workflow events are incompatible with | ||
| Temporal Ruby workflow events at runtime, so a live workflow migration cannot occur, an alternative task queue would be |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think more information could be helpful here. What does a closed workflow migration look like? Why is an alternative task queue needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't have a specific answer here on how they migrate a workflow from one set of code to another (whether that's for code versioning reasons or SDK change reasons (be it this one or switching languages)). I think that's more of a whole-workflow-versioning problem I would rather not delve into here and leave that to whole-workflow-versioning docs. I will see if there are whole-workflow-versioning docs I can link to (probably https://docs.temporal.io/production-deployment/worker-deployments/worker-versioning).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sounds good, maybe worth removing "an alternative task queue needed" bit? Not sure what value that adds, unless we dive further into what that statement means
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think knowing that you can just put new stuff on an alternative task queue is plenty valuable on its own. But I may be able to add a bit more to the sentence.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added some more detail here
| | Handle signal/query/update in workflow | `workflow.on_signal`/`workflow.on_query`/update-unsupported | Decorate with `workflow_signal`/`workflow_query`/`workflow_update` | | ||
| | Run worker | `Temporal::Worker.new` + `start` | `Temporalio::Worker.new` + `run` | | ||
|
|
||
| This is just a high-level overview, there are many more differences on more specific Temporal components. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there already info on the more specific differences, or does that still need to be written
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are thousands (...er...hundreds maybe heh) of differences from option names to method names to interceptor approaches, we don't want to write every difference. This is a small documentation section as a courtesy for a handful of non-Temporal-Ruby-SDK users. It'd be like documenting every difference between Go and Java SDKs just to help users do a Go-to-Java migration.
| how to disable API class loading on the Coinbase Ruby side if needing to use both dependencies in the same project since | ||
| two sets of API classes cannot both be present. | ||
|
|
||
| Migration cannot be done on a live, running workflow. Overall, Coinbase Ruby workflow events are incompatible with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it redundant to say "live" and "running"? Is there ever a running workflow that isn't "live"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically yes, a workflow running on the server may never have reached worker code. Also there are "live" workflows that are not running (e.g. closed workflows that get queried). But in general, both adjectives are needed to clarify that you can't do the migration with live code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sounds good, thanks
What was changed
RENDERED (as of this PR, will go away when PR merged)
Added fairly limited section on migration from Coinbase Ruby which notes the sample, that workflows are incompatible, and a table showing the differences for commonly used features.